repo/commit: Split up metadata/content commit paths
authorColin Walters <walters@verbum.org>
Tue, 23 May 2017 20:18:31 +0000 (16:18 -0400)
committerAtomic Bot <atomic-devel@projectatomic.io>
Thu, 1 Jun 2017 18:43:38 +0000 (18:43 +0000)
There was a lot of conditionals inside `write_object()` differentating
between metadata/content, and then for content, on the different repo
types.  Further, in the metadata path since the logic is simpler, can
present a non-streaming API, and further use `OtTmpfile`, etc.

Splitting them up helps drop a lot of conditionals. We introduce a small
`CleanupUnlinkat` that allows us to fully convert to the new code style in both
functions.

This itself is still prep for fully switching to `GLnxTmpfile`.

Closes: #881
Approved by: jlebon

src/libostree/ostree-repo-commit.c

index 226ddcdadf3ce4bb5fdcc678899e96eb245becc9..870a06c71be3a92e90d22a336a95a4ee9f2b3429 100644 (file)
@@ -591,292 +591,356 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self,
   return TRUE;
 }
 
-static gboolean
-write_object (OstreeRepo         *self,
-              OstreeObjectType    objtype,
-              const char         *expected_checksum,
-              GInputStream       *input,
-              guint64             file_object_length,
-              guchar            **out_csum,
-              GCancellable       *cancellable,
-              GError            **error)
+/* A little helper to call unlinkat() as a cleanup
+ * function.  Mostly only necessary to handle
+ * deletion of temporary symlinks.
+ */
+typedef struct {
+  int dfd;
+  const char *path;
+} CleanupUnlinkat;
+
+static void
+cleanup_unlinkat (CleanupUnlinkat *cleanup)
 {
-  gboolean ret = FALSE;
-  const char *actual_checksum = NULL;
-  g_autofree char *actual_checksum_owned = NULL;
-  gboolean do_commit;
-  OstreeRepoMode repo_mode;
-  g_autofree char *temp_filename = NULL;
-  g_autofree guchar *ret_csum = NULL;
-  glnx_unref_object OtChecksumInstream *checksum_input = NULL;
-  g_autoptr(GInputStream) file_input = NULL;
-  g_autoptr(GFileInfo) file_info = NULL;
-  g_autoptr(GVariant) xattrs = NULL;
-  gboolean have_obj;
-  gboolean temp_file_is_regular;
-  gboolean temp_file_is_symlink;
-  glnx_fd_close int temp_fd = -1;
-  gboolean object_is_symlink = FALSE;
-  gssize unpacked_size = 0;
-  gboolean indexable = FALSE;
+  if (cleanup->path)
+    (void) unlinkat (cleanup->dfd, cleanup->path, 0);
+}
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(CleanupUnlinkat, cleanup_unlinkat);
 
+/* Write a content object. */
+static gboolean
+write_content_object (OstreeRepo         *self,
+                      const char         *expected_checksum,
+                      GInputStream       *input,
+                      guint64             file_object_length,
+                      guchar            **out_csum,
+                      GCancellable       *cancellable,
+                      GError            **error)
+{
   g_return_val_if_fail (expected_checksum || out_csum, FALSE);
 
   if (g_cancellable_set_error_if_cancelled (cancellable, error))
     return FALSE;
 
-  repo_mode = ostree_repo_get_mode (self);
+  OstreeRepoMode repo_mode = ostree_repo_get_mode (self);
 
+  glnx_unref_object OtChecksumInstream *checksum_input = NULL;
   if (out_csum)
     checksum_input = ot_checksum_instream_new (input, G_CHECKSUM_SHA256);
 
-  if (objtype == OSTREE_OBJECT_TYPE_FILE)
-    {
-      if (!ostree_content_stream_parse (FALSE, checksum_input ? (GInputStream*)checksum_input : input,
-                                        file_object_length, FALSE,
-                                        &file_input, &file_info, &xattrs,
-                                        cancellable, error))
-        goto out;
+  g_autoptr(GInputStream) file_input = NULL;
+  g_autoptr(GVariant) xattrs = NULL;
+  g_autoptr(GFileInfo) file_info = NULL;
+  if (!ostree_content_stream_parse (FALSE, checksum_input ? (GInputStream*)checksum_input : input,
+                                    file_object_length, FALSE,
+                                    &file_input, &file_info, &xattrs,
+                                    cancellable, error))
+    return FALSE;
 
-      temp_file_is_regular = g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR;
-      temp_file_is_symlink = object_is_symlink =
-        g_file_info_get_file_type (file_info) == G_FILE_TYPE_SYMBOLIC_LINK;
+  gboolean temp_file_is_regular = g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR;
+  gboolean temp_file_is_symlink = g_file_info_get_file_type (file_info) == G_FILE_TYPE_SYMBOLIC_LINK;
+  gboolean object_is_symlink = temp_file_is_symlink;
 
-      if (repo_mode == OSTREE_REPO_MODE_BARE_USER && object_is_symlink)
-        {
-          const char *target_str = g_file_info_get_symlink_target (file_info);
-          g_autoptr(GBytes) target = g_bytes_new (target_str, strlen (target_str) + 1);
-
-          /* For bare-user we can't store symlinks as symlinks, as symlinks don't
-             support user xattrs to store the ownership. So, instead store them
-             as regular files */
-          temp_file_is_regular = TRUE;
-          temp_file_is_symlink = FALSE;
-          if (file_input != NULL)
-            g_object_unref (file_input);
-
-          /* Include the terminating zero so we can e.g. mmap this file */
-          file_input = g_memory_input_stream_new_from_bytes (target);
-        }
+  if (repo_mode == OSTREE_REPO_MODE_BARE_USER && object_is_symlink)
+    {
+      const char *target_str = g_file_info_get_symlink_target (file_info);
+      g_autoptr(GBytes) target = g_bytes_new (target_str, strlen (target_str) + 1);
 
-      if (!(temp_file_is_regular || temp_file_is_symlink))
-        {
-          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                       "Unsupported file type %u", g_file_info_get_file_type (file_info));
-          goto out;
-        }
+      /* For bare-user we can't store symlinks as symlinks, as symlinks don't
+         support user xattrs to store the ownership. So, instead store them
+         as regular files */
+      temp_file_is_regular = TRUE;
+      temp_file_is_symlink = FALSE;
 
-      /* For regular files, we create them with default mode, and only
-       * later apply any xattrs and setuid bits.  The rationale here
-       * is that an attacker on the network with the ability to MITM
-       * could potentially cause the system to make a temporary setuid
-       * binary with trailing garbage, creating a window on the local
-       * system where a malicious setuid binary exists.
-       */
-      if ((_ostree_repo_mode_is_bare (repo_mode)) && temp_file_is_regular)
-        {
-          guint64 size = g_file_info_get_size (file_info);
+      if (file_input != NULL)
+        g_object_unref (file_input);
+      /* Include the terminating zero so we can e.g. mmap this file */
+      file_input = g_memory_input_stream_new_from_bytes (target);
+    }
 
-          if (!create_regular_tmpfile_linkable_with_content (self, size, file_input,
-                                                             &temp_fd, &temp_filename,
-                                                             cancellable, error))
-            goto out;
-        }
-      else if (_ostree_repo_mode_is_bare (repo_mode) && temp_file_is_symlink)
-        {
-          /* Note: This will not be hit for bare-user mode because its converted to a
-             regular file and take the branch above */
-          if (!_ostree_make_temporary_symlink_at (self->tmp_dir_fd,
-                                                  g_file_info_get_symlink_target (file_info),
-                                                  &temp_filename,
-                                                  cancellable, error))
-            goto out;
-        }
-      else if (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
-        {
-          g_autoptr(GVariant) file_meta = NULL;
-          g_autoptr(GConverter) zlib_compressor = NULL;
-          g_autoptr(GOutputStream) compressed_out_stream = NULL;
-          g_autoptr(GOutputStream) temp_out = NULL;
+  if (!(temp_file_is_regular || temp_file_is_symlink))
+    return glnx_throw (error, "Unsupported file type %u", g_file_info_get_file_type (file_info));
 
-          if (self->generate_sizes)
-            indexable = TRUE;
+  /* For regular files, we create them with default mode, and only
+   * later apply any xattrs and setuid bits.  The rationale here
+   * is that an attacker on the network with the ability to MITM
+   * could potentially cause the system to make a temporary setuid
+   * binary with trailing garbage, creating a window on the local
+   * system where a malicious setuid binary exists.
+   */
+  /* These variables are almost equivalent to OtTmpfile, except
+   * temp_filename might also be a symlink.  Hence the CleanupUnlinkat
+   * which handles that case.
+   */
+  g_auto(CleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL };
+  glnx_fd_close int temp_fd = -1;
+  g_autofree char *temp_filename = NULL;
+  gssize unpacked_size = 0;
+  gboolean indexable = FALSE;
+  if ((_ostree_repo_mode_is_bare (repo_mode)) && temp_file_is_regular)
+    {
+      guint64 size = g_file_info_get_size (file_info);
 
-          if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
-                                              &temp_fd, &temp_filename,
-                                              error))
-            goto out;
-          temp_file_is_regular = TRUE;
-          temp_out = g_unix_output_stream_new (temp_fd, FALSE);
+      if (!create_regular_tmpfile_linkable_with_content (self, size, file_input,
+                                                         &temp_fd, &temp_filename,
+                                                         cancellable, error))
+        return FALSE;
+      tmp_unlinker.path = temp_filename;
+    }
+  else if (_ostree_repo_mode_is_bare (repo_mode) && temp_file_is_symlink)
+    {
+      /* Note: This will not be hit for bare-user mode because its converted to a
+         regular file and take the branch above */
+      if (!_ostree_make_temporary_symlink_at (self->tmp_dir_fd,
+                                              g_file_info_get_symlink_target (file_info),
+                                              &temp_filename,
+                                              cancellable, error))
+        return FALSE;
+      tmp_unlinker.path = temp_filename;
+    }
+  else if (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
+    {
+      g_autoptr(GVariant) file_meta = NULL;
+      g_autoptr(GConverter) zlib_compressor = NULL;
+      g_autoptr(GOutputStream) compressed_out_stream = NULL;
+      g_autoptr(GOutputStream) temp_out = NULL;
 
-          file_meta = _ostree_zlib_file_header_new (file_info, xattrs);
+      if (self->generate_sizes)
+        indexable = TRUE;
 
-          if (!_ostree_write_variant_with_size (temp_out, file_meta, 0, NULL, NULL,
-                                                cancellable, error))
-            goto out;
+      if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
+                                          &temp_fd, &temp_filename,
+                                          error))
+        return FALSE;
+      tmp_unlinker.path = temp_filename;
+      temp_file_is_regular = TRUE;
+      temp_out = g_unix_output_stream_new (temp_fd, FALSE);
 
-          if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
-            {
-              zlib_compressor = (GConverter*)g_zlib_compressor_new (G_ZLIB_COMPRESSOR_FORMAT_RAW, self->zlib_compression_level);
-              compressed_out_stream = g_converter_output_stream_new (temp_out, zlib_compressor);
-              /* Don't close the base; we'll do that later */
-              g_filter_output_stream_set_close_base_stream ((GFilterOutputStream*)compressed_out_stream, FALSE);
-              
-              unpacked_size = g_output_stream_splice (compressed_out_stream, file_input,
-                                                      0, cancellable, error);
-              if (unpacked_size < 0)
-                goto out;
-            }
+      file_meta = _ostree_zlib_file_header_new (file_info, xattrs);
 
-          if (!g_output_stream_flush (temp_out, cancellable, error))
-            goto out;
+      if (!_ostree_write_variant_with_size (temp_out, file_meta, 0, NULL, NULL,
+                                            cancellable, error))
+        return FALSE;
 
-          if (fchmod (temp_fd, 0644) < 0)
-            {
-              glnx_set_error_from_errno (error);
-              goto out;
-            }
+      if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
+        {
+          zlib_compressor = (GConverter*)g_zlib_compressor_new (G_ZLIB_COMPRESSOR_FORMAT_RAW, self->zlib_compression_level);
+          compressed_out_stream = g_converter_output_stream_new (temp_out, zlib_compressor);
+          /* Don't close the base; we'll do that later */
+          g_filter_output_stream_set_close_base_stream ((GFilterOutputStream*)compressed_out_stream, FALSE);
+
+          unpacked_size = g_output_stream_splice (compressed_out_stream, file_input,
+                                                  0, cancellable, error);
+          if (unpacked_size < 0)
+            return FALSE;
         }
-      else
-        g_assert_not_reached ();
-    }
-  else
-    {
-      if (!create_regular_tmpfile_linkable_with_content (self, file_object_length,
-                                                         checksum_input ? (GInputStream*)checksum_input : input,
-                                                         &temp_fd, &temp_filename,
-                                                         cancellable, error))
-        goto out;
-      temp_file_is_regular = TRUE;
+
+      if (!g_output_stream_flush (temp_out, cancellable, error))
+        return FALSE;
+
+      if (fchmod (temp_fd, 0644) < 0)
+        return glnx_throw_errno_prefix (error, "fchmod");
     }
 
+  const char *actual_checksum = NULL;
+  g_autofree char *actual_checksum_owned = NULL;
   if (!checksum_input)
     actual_checksum = expected_checksum;
   else
     {
       actual_checksum = actual_checksum_owned = ot_checksum_instream_get_string (checksum_input);
       if (expected_checksum && strcmp (actual_checksum, expected_checksum) != 0)
-        {
-          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                       "Corrupted %s object %s (actual checksum is %s)",
-                       ostree_object_type_to_string (objtype),
-                       expected_checksum, actual_checksum);
-          goto out;
-        }
+        return glnx_throw (error, "Corrupted %s object %s (actual checksum is %s)",
+                           ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE),
+                           expected_checksum, actual_checksum);
     }
 
   g_assert (actual_checksum != NULL); /* Pacify static analysis */
-          
+
+  /* See whether or not we have the object, now that we know the
+   * checksum.
+   */
+  gboolean have_obj;
+  if (!_ostree_repo_has_loose_object (self, actual_checksum, OSTREE_OBJECT_TYPE_FILE,
+                                      &have_obj, cancellable, error))
+    return FALSE;
+  /* If we already have it, just update the stats. */
+  if (have_obj)
+    {
+      g_mutex_lock (&self->txn_stats_lock);
+      self->txn_stats.content_objects_total++;
+      g_mutex_unlock (&self->txn_stats_lock);
+      if (out_csum)
+        *out_csum = ostree_checksum_to_bytes (actual_checksum);
+      /* Note early return */
+      return TRUE;
+    }
+
+  const guint32 uid = g_file_info_get_attribute_uint32 (file_info, "unix::uid");
+  const guint32 gid = g_file_info_get_attribute_uint32 (file_info, "unix::gid");
+  const guint32 mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
+  if (!commit_loose_object_trusted (self, actual_checksum,
+                                    OSTREE_OBJECT_TYPE_FILE,
+                                    temp_filename,
+                                    object_is_symlink,
+                                    uid, gid, mode,
+                                    xattrs, temp_fd,
+                                    cancellable, error))
+    return FALSE;
+  /* Clear the unlinker path, it was consumed */
+  tmp_unlinker.path = NULL;
+
+  /* Update size metadata if configured */
   if (indexable && temp_file_is_regular)
     {
       struct stat stbuf;
 
-      if (fstat (temp_fd, &stbuf) == -1)
-        {
-          glnx_set_error_from_errno (error);
-          goto out;
-        }
+      if (!glnx_fstat (temp_fd, &stbuf, error))
+        return FALSE;
 
       repo_store_size_entry (self, actual_checksum, unpacked_size, stbuf.st_size);
     }
 
-  if (!_ostree_repo_has_loose_object (self, actual_checksum, objtype, &have_obj,
-                                      cancellable, error))
-    goto out;
-          
-  do_commit = !have_obj;
+  /* Update statistics */
+  g_mutex_lock (&self->txn_stats_lock);
+  self->txn_stats.content_objects_written++;
+  self->txn_stats.content_bytes_written += file_object_length;
+  self->txn_stats.content_objects_total++;
+  g_mutex_unlock (&self->txn_stats_lock);
 
-  if (do_commit)
+  if (out_csum)
     {
-      guint32 uid, gid, mode;
+      g_assert (actual_checksum);
+      *out_csum = ostree_checksum_to_bytes (actual_checksum);
+    }
 
-      if (file_info)
-        {
-          uid = g_file_info_get_attribute_uint32 (file_info, "unix::uid");
-          gid = g_file_info_get_attribute_uint32 (file_info, "unix::gid");
-          mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
-        }
-      else
-        uid = gid = mode = 0;
-      
-      if (!commit_loose_object_trusted (self, actual_checksum, objtype,
-                                        temp_filename,
-                                        object_is_symlink,
-                                        uid, gid, mode,
-                                        xattrs, temp_fd,
-                                        cancellable, error))
-        goto out;
+  return TRUE;
+}
 
+static gboolean
+write_metadata_object (OstreeRepo         *self,
+                       OstreeObjectType    objtype,
+                       const char         *expected_checksum,
+                       GBytes             *buf,
+                       guchar            **out_csum,
+                       GCancellable       *cancellable,
+                       GError            **error)
+{
+  g_return_val_if_fail (expected_checksum || out_csum, FALSE);
 
-      if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
-        {
-          GError *local_error = NULL;
-          /* If we are writing a commit, be sure there is no tombstone for it.
-             We may have deleted the commit and now we are trying to pull it again.  */
-          if (!ostree_repo_delete_object (self,
-                                          OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT,
-                                          actual_checksum,
-                                          cancellable,
-                                          &local_error))
-            {
-              if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
-                g_clear_error (&local_error);
-              else
-                {
-                  g_propagate_error (error, local_error);
-                  goto out;
-                }
-            }
-        }
+  if (g_cancellable_set_error_if_cancelled (cancellable, error))
+    return FALSE;
 
-      if (OSTREE_OBJECT_TYPE_IS_META (objtype))
+  /* In the metadata case, we're not streaming, so we don't bother creating a
+   * tempfile until we compute the checksum. Some metadata like dirmeta is
+   * commonly duplicated, and computing the checksum is going to be cheaper than
+   * making a tempfile.
+   *
+   * However, tombstone commit types don't make sense to checksum, because for
+   * historical reasons we used ostree_repo_write_metadata_trusted() with the
+   * *original* sha256 to say what commit was being killed.
+   */
+  const gboolean is_tombstone = (objtype == OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT);
+  g_autofree char *actual_checksum = NULL;
+  if (is_tombstone)
+    {
+      actual_checksum = g_strdup (expected_checksum);
+    }
+  else
+    {
+      actual_checksum = g_compute_checksum_for_bytes (G_CHECKSUM_SHA256, buf);
+      gboolean have_obj;
+      if (!_ostree_repo_has_loose_object (self, actual_checksum, objtype, &have_obj,
+                                          cancellable, error))
+        return FALSE;
+      /* If we already have the object, we just need to update the tried-to-commit
+       * stat for metadata and be done here.
+       */
+      if (have_obj)
         {
-          if (G_UNLIKELY (file_object_length > OSTREE_MAX_METADATA_WARN_SIZE))
-            {
-              g_autofree char *metasize = g_format_size (file_object_length);
-              g_autofree char *warnsize = g_format_size (OSTREE_MAX_METADATA_WARN_SIZE);
-              g_autofree char *maxsize = g_format_size (OSTREE_MAX_METADATA_SIZE);
-              g_warning ("metadata object %s is %s, which is larger than the warning threshold of %s." \
-                         "  The hard limit on metadata size is %s.  Put large content in the tree itself, not in metadata.",
-                         actual_checksum,
-                         metasize, warnsize, maxsize);
-            }
+          g_mutex_lock (&self->txn_stats_lock);
+          self->txn_stats.metadata_objects_total++;
+          g_mutex_unlock (&self->txn_stats_lock);
+
+          if (out_csum)
+            *out_csum = ostree_checksum_to_bytes (actual_checksum);
+          /* Note early return */
+          return TRUE;
         }
 
-      g_clear_pointer (&temp_filename, g_free);
+      if (expected_checksum && strcmp (actual_checksum, expected_checksum) != 0)
+        return glnx_throw (error, "Corrupted %s object %s (actual checksum is %s)",
+                           ostree_object_type_to_string (objtype),
+                           expected_checksum, actual_checksum);
     }
 
-  g_mutex_lock (&self->txn_stats_lock);
-  if (do_commit)
+  /* Ok, checksum is known, let's get the data */
+  gsize len;
+  const guint8 *bufp = g_bytes_get_data (buf, &len);
+
+  /* Do the size warning here, to avoid warning for already extant metadata */
+  if (G_UNLIKELY (len > OSTREE_MAX_METADATA_WARN_SIZE))
     {
-      if (OSTREE_OBJECT_TYPE_IS_META (objtype))
-        {
-          self->txn_stats.metadata_objects_written++;
-        }
-      else
+      g_autofree char *metasize = g_format_size (len);
+      g_autofree char *warnsize = g_format_size (OSTREE_MAX_METADATA_WARN_SIZE);
+      g_autofree char *maxsize = g_format_size (OSTREE_MAX_METADATA_SIZE);
+      g_warning ("metadata object %s is %s, which is larger than the warning threshold of %s." \
+                 "  The hard limit on metadata size is %s.  Put large content in the tree itself, not in metadata.",
+                 actual_checksum,
+                 metasize, warnsize, maxsize);
+    }
+
+  /* Write the metadata to a temporary file */
+  g_auto(OtTmpfile) tmpf = { 0, };
+  if (!ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
+                                    &tmpf, error))
+    return FALSE;
+  if (!ot_fallocate (tmpf.fd, len, error))
+    return FALSE;
+  if (glnx_loop_write (tmpf.fd, bufp, len) < 0)
+    return glnx_throw_errno_prefix (error, "write()");
+  if (fchmod (tmpf.fd, 0644) < 0)
+    return glnx_throw_errno_prefix (error, "fchmod");
+
+  /* And commit it into place */
+  if (!_ostree_repo_commit_loose_final (self, actual_checksum, objtype,
+                                        self->tmp_dir_fd, tmpf.fd, tmpf.path,
+                                        cancellable, error))
+    return FALSE;
+  /* The temp path was consumed */
+  g_clear_pointer (&tmpf.path, g_free);
+
+  if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
+    {
+      GError *local_error = NULL;
+      /* If we are writing a commit, be sure there is no tombstone for it.
+         We may have deleted the commit and now we are trying to pull it again.  */
+      if (!ostree_repo_delete_object (self,
+                                      OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT,
+                                      actual_checksum,
+                                      cancellable,
+                                      &local_error))
         {
-          self->txn_stats.content_objects_written++;
-          self->txn_stats.content_bytes_written += file_object_length;
+          if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
+            g_clear_error (&local_error);
+          else
+            {
+              g_propagate_error (error, local_error);
+              return FALSE;
+            }
         }
     }
-  if (OSTREE_OBJECT_TYPE_IS_META (objtype))
-    self->txn_stats.metadata_objects_total++;
-  else
-    self->txn_stats.content_objects_total++;
-  g_mutex_unlock (&self->txn_stats_lock);
 
-  if (checksum_input)
-    {
-      g_assert (actual_checksum);
-      ret_csum = ostree_checksum_to_bytes (actual_checksum);
-    }
+  /* Update the stats, note we both wrote one and add to total */
+  g_mutex_lock (&self->txn_stats_lock);
+  self->txn_stats.metadata_objects_written++;
+  self->txn_stats.metadata_objects_total++;
+  g_mutex_unlock (&self->txn_stats_lock);
 
-  ret = TRUE;
-  ot_transfer_out_value(out_csum, &ret_csum);
- out:
-  if (temp_filename)
-    (void) unlinkat (self->tmp_dir_fd, temp_filename, 0);
-  return ret;
+  if (out_csum)
+    *out_csum = ostree_checksum_to_bytes (actual_checksum);
+  return TRUE;
 }
 
 static gboolean
@@ -1541,11 +1605,9 @@ ostree_repo_write_metadata (OstreeRepo         *self,
   if (!metadata_size_valid (objtype, g_variant_get_size (normalized), error))
     return FALSE;
 
-  g_autoptr(GInputStream) input = ot_variant_read (normalized);
-  if (!write_object (self, objtype, expected_checksum,
-                     input, g_variant_get_size (normalized),
-                     out_csum,
-                     cancellable, error))
+  g_autoptr(GBytes) vdata = g_variant_get_data_as_bytes (normalized);
+  if (!write_metadata_object (self, objtype, expected_checksum,
+                              vdata, out_csum, cancellable, error))
     return FALSE;
 
   return TRUE;
@@ -1805,9 +1867,9 @@ ostree_repo_write_content (OstreeRepo       *self,
         }
     }
 
-  return write_object (self, OSTREE_OBJECT_TYPE_FILE, expected_checksum,
-                       object_input, length, out_csum,
-                       cancellable, error);
+  return write_content_object (self, expected_checksum,
+                               object_input, length, out_csum,
+                               cancellable, error);
 }
 
 typedef struct {